Make particle gun parameters configurable and add tracking scan#1158
Make particle gun parameters configurable and add tracking scan#1158olantwin wants to merge 4 commits into
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe changes extend a tracking benchmark suite to support configurable polar-angle ranges and multiple-track generation. A new scan script orchestrates benchmarks across angle and multiplicity parameter grids, while the metrics module now handles charge identification and properly represents undefined metric values when denominators are zero. Changes
Sequence DiagramsequenceDiagram
actor User
participant CLI as run_tracking_scan.py
participant Benchmark as run_tracking_benchmark.py
participant Sim as run_simScript.py
participant Metrics as tracking_benchmark.py
participant Results as Result Files
User->>CLI: run with scan parameters
loop Per scan point (theta, nTracks)
CLI->>CLI: Compute scan point parameters
CLI->>Benchmark: Launch with theta bounds & nTracks
Benchmark->>Sim: Invoke simulation subprocess
Sim->>Sim: Generate particles with (theta, nTracks, mixCharges)
Sim->>Metrics: Create tracking_metrics.json
Benchmark->>Metrics: Parse output/metrics
Benchmark->>Results: Write point JSON
CLI->>CLI: Load tracking_metrics.json
CLI->>CLI: Extract & append to scan results
end
CLI->>Results: Write scan_results.json
CLI->>Results: Generate ROOT TGraphErrors (scan_summary.root)
CLI->>Results: Generate matplotlib plots (optional)
CLI->>User: Report scan completion & output paths
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Add --thetaMin, --thetaMax, and --nTracks arguments to the PG subcommand in run_simScript.py (previously hardcoded to theta=0, nTracks=1). Wire these through run_tracking_benchmark.py so the tracking benchmark can be run at arbitrary angles and multiplicities.
New script run_tracking_scan.py that sweeps the tracking benchmark over a grid of polar angles and track multiplicities. Includes charge-ID efficiency, --mixCharges flag, null handling for undefined metrics, points-only plots with density annotation.
103f7bb to
16fb7fa
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
macro/run_tracking_scan.py (2)
155-156: Minor duplication:gun_area_m2computed in two places.The gun area calculation
200 * 300 * 1e-4appears at lines 156 and 284. Consider extracting it to a constant near the top of the file.♻️ Extract constant
+# Gun area from benchmark defaults: Dx=200cm, Dy=300cm → m² +GUN_AREA_M2 = 200 * 300 * 1e-4 # ... in the scan loop: - gun_area_m2 = 200 * 300 * 1e-4 all_results.append( { ... - "density_per_m2": round(n_tracks / gun_area_m2, 4), + "density_per_m2": round(n_tracks / GUN_AREA_M2, 4), # ... in matplotlib section: - gun_area_m2 = 200 * 300 * 1e-4 # Dx=200, Dy=300 cm + # GUN_AREA_M2 already defined at module levelAlso applies to: 284-284
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@macro/run_tracking_scan.py` around lines 155 - 156, The calculation for gun_area_m2 is duplicated; extract the value into a single constant (e.g., GUN_AREA_M2) defined near the top of the module and replace both occurrences of the literal expression (200 * 300 * 1e-4) with that constant, updating uses of gun_area_m2 in the functions/blocks that reference it so they read from GUN_AREA_M2 and removing the duplicate local computation.
152-166: Consider adding error handling for JSON loading.If the benchmark subprocess fails after creating but before completing the JSON file,
json.loadwill raise an exception. While unlikely, adding a try-except would make the scan more robust.♻️ Optional defensive check
- with open(json_path) as f: - metrics = json.load(f) + try: + with open(json_path) as f: + metrics = json.load(f) + except (json.JSONDecodeError, KeyError) as e: + print(f"WARNING: Could not parse metrics from {json_path}: {e}") + continue🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@macro/run_tracking_scan.py` around lines 152 - 166, The json.load call reading json_path can raise exceptions if the subprocess wrote an incomplete or missing file; wrap the load in a try/except that catches FileNotFoundError, json.JSONDecodeError (or json.decoder.JSONDecodeError) and any other Exception, log a clear warning including json_path and the exception, and either skip appending this result (or set metrics = {"tracking_benchmark": {}} / a safe default) so that the subsequent all_results.append using metrics["tracking_benchmark"] cannot crash; update the block around json.load, json_path, metrics and the all_results.append to use the safe metrics value or early-continue on error.macro/run_simScript.py (1)
524-527: Consider validating particle type when--mixChargesis enabled.The charge mixing logic assumes the particle has a valid antiparticle with PDG code
-abs(pID). This works for charged particles like muons (±13) but would produce invalid PDG codes for neutral particles (e.g., photon 22 → -22 is undefined).Consider adding a validation check or at least documenting that
--mixChargesis only valid for charged particles.♻️ Optional validation
if options.command == "PG": + if options.mixCharges and options.nTracks > 1: + # Verify particle has a valid antiparticle (charged particle) + PDG = ROOT.TDatabasePDG.Instance() + particle = PDG.GetParticle(abs(options.pID)) + if particle is None or particle.Charge() == 0: + print(f"WARNING: --mixCharges requested but pID={options.pID} is not a charged particle") if options.mixCharges and options.nTracks > 1: pids = [(abs(options.pID), options.nTracks // 2), (-abs(options.pID), (options.nTracks + 1) // 2)]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@macro/run_simScript.py` around lines 524 - 527, When options.mixCharges is true, validate that the chosen particle is actually charged before constructing pids: check options.pID and ensure abs(options.pID) is not a known neutral PDG (e.g., 22, 111, 130, 2112) or, if a particle lookup API exists in the codebase, use it to confirm the particle has an antiparticle; if the check fails, raise/exit with a clear error or disable mixCharges. Update the branch that sets pids (references: options.mixCharges, options.pID, pids, options.nTracks) to perform this validation first and only produce the two-charge split when the particle is validated as charged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@python/tracking_benchmark.py`:
- Around line 300-314: The current broad "except Exception: pass" around the
charge-identification block (involving fitted_state.getCharge(),
self.sim_tree.MCTrack[mc_id].GetPdgCode(), self.PDG.GetParticle and mc_particle)
silently swallows errors—replace it so exceptions are either narrowed to the
specific GenFit/expected exception type or captured as "except Exception as e"
and logged (e.g., using the module logger or logging.exception) with context
like mc_id and p_truth to aid debugging, rather than silently passing.
---
Nitpick comments:
In `@macro/run_simScript.py`:
- Around line 524-527: When options.mixCharges is true, validate that the chosen
particle is actually charged before constructing pids: check options.pID and
ensure abs(options.pID) is not a known neutral PDG (e.g., 22, 111, 130, 2112)
or, if a particle lookup API exists in the codebase, use it to confirm the
particle has an antiparticle; if the check fails, raise/exit with a clear error
or disable mixCharges. Update the branch that sets pids (references:
options.mixCharges, options.pID, pids, options.nTracks) to perform this
validation first and only produce the two-charge split when the particle is
validated as charged.
In `@macro/run_tracking_scan.py`:
- Around line 155-156: The calculation for gun_area_m2 is duplicated; extract
the value into a single constant (e.g., GUN_AREA_M2) defined near the top of the
module and replace both occurrences of the literal expression (200 * 300 * 1e-4)
with that constant, updating uses of gun_area_m2 in the functions/blocks that
reference it so they read from GUN_AREA_M2 and removing the duplicate local
computation.
- Around line 152-166: The json.load call reading json_path can raise exceptions
if the subprocess wrote an incomplete or missing file; wrap the load in a
try/except that catches FileNotFoundError, json.JSONDecodeError (or
json.decoder.JSONDecodeError) and any other Exception, log a clear warning
including json_path and the exception, and either skip appending this result (or
set metrics = {"tracking_benchmark": {}} / a safe default) so that the
subsequent all_results.append using metrics["tracking_benchmark"] cannot crash;
update the block around json.load, json_path, metrics and the all_results.append
to use the safe metrics value or early-continue on error.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 55c97e0c-de7c-4758-bba0-d0398a546483
📒 Files selected for processing (5)
CHANGELOG.mdmacro/run_simScript.pymacro/run_tracking_benchmark.pymacro/run_tracking_scan.pypython/tracking_benchmark.py
- Log fitted-state exceptions instead of silently swallowing them - Validate particle is charged before applying mixCharges - Extract duplicated gun_area_m2 to module-level GUN_AREA_M2 constant - Wrap json.load in error handling for corrupt/missing result files
|
Will be partially superseded by #1183 |
--thetaMin,--thetaMax,--nTracksrun_tracking_scan.pyto sweep tracking benchmark over angle and multiplicity grids--mixChargesflag for particle/antiparticle generationChecklist
Summary by CodeRabbit
New Features
Bug Fixes